Skip to content

fix: decode macOS .mobileconfig payloads, stop decoding script hashes - #42

Merged
royklo merged 4 commits into
mainfrom
fix/decode-macos-payloads-and-hashes
Sep 11, 2026
Merged

fix: decode macOS .mobileconfig payloads, stop decoding script hashes#42
royklo merged 4 commits into
mainfrom
fix/decode-macos-payloads-and-hashes

Conversation

@royklo

@royklo royklo commented Sep 11, 2026

Copy link
Copy Markdown
Owner

Three rendering bugs, found by reading two generated reports.

  • macOS/iOS profiles now show what's in them. payload is a base64 .mobileconfig
    (a plist XML document) and the reports printed the base64 verbatim — 2,300 characters
    that told you a profile existed and nothing about what it configured.
  • Script hashes are no longer decoded. hashedScriptContent is a raw digest; the
    decoder matched it on name alongside scriptContent and turned it into
    V<FFFD><FFFD>0…, presented behind a "View script" toggle.
  • Code blocks scroll horizontally instead of re-wrapping and breaking lines
    mid-identifier. overflow-x: auto was already declared in both renderers — it was
    dead as long as white-space: pre-wrap sat in front of it.

Shared root cause of the first two: the property name was the only test.
ConvertFrom-InforcerBase64Text now decodes with strict UTF-8 and rejects control bytes,
putting the guard on the decoded bytes. That also covers signed profiles and any future
binary field. The helper replaces all four duplicate inline decode sites.

Turned up while scanning for more of the same: the internal __SCRIPT_CODE__ render
marker leaked into the Markdown and Excel exports, and newlines in decoded content end
the row in a GFM table — silently destroying the rest of the settings table for any
policy with script content.

Docs: module/README.md still said 20 cmdlets (the manifest exports 21 and omitted
Get-InforcerSecureScore), scope labels in API-REFERENCE.md normalised to one spelling,
dead #code-style anchor in CONTRIBUTING.md.

448/0/2 tests, 13 new. Version 0.7.1 — PATCH, the public surface is unchanged.

royklo and others added 3 commits September 11, 2026 11:13
Three findings from reading two generated documentation reports.

macOS/iOS custom configuration profiles printed the `payload` field as raw
base64 rather than the .mobileconfig plist it encodes, so a profile like
"macOS - Security - D - Google Chrome - Extensions" showed 2,300 characters
of base64 and never revealed which extensions it allowed.

The decoder matched `hashedScriptContent` on name alongside `scriptContent`
and UTF8-decoded the digest into replacement characters, presenting
"V<FFFD><FFFD>0<FFFD>..." as viewable script content.

Both come from the same defect: the property name was the only test. The new
`ConvertFrom-InforcerBase64Text` decodes with strict UTF-8 and rejects control
bytes, returning $null so callers keep the base64 — the guard is now on the
decoded bytes, which also covers signed profiles and any future binary field.
It replaces all four duplicate inline decode sites, each of which gained
`^payload$`. Both HTML renderers render decoded plists in an `xml-code` block.

Scanning for more of the same turned up a third: the internal `__SCRIPT_CODE__`
marker leaked into the Markdown and Excel exports, and newlines in decoded
content ended the row mid-table in GFM, destroying the rest of the settings
table for any policy with a script.

448/0/2 tests, 13 new.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
white-space: pre-wrap plus word-break: break-all re-flowed every code block to
the column width and broke lines mid-identifier — it flattened the nesting of a
decoded plist and split shell variables in half.

Both renderers already declared overflow-x: auto, which was dead as long as the
content wrapped. Horizontal scrolling was always the intent; it just never took
effect. Fixed on the shared .script-collapsible pre rule rather than on the new
.xml-code class, because scripts and JSON were being mangled the same way.

Safe inside the layouts that hold these blocks: the doc renderer's table.t2 is
table-layout: fixed with 50/50 columns, and the comparison renderer's Manual
Review cells already carry min-width: 0 and max-width: 100%, so a long line
scrolls within the block rather than widening the table.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
module/README.md still described 20 exported cmdlets and omitted
Get-InforcerSecureScore from the Public/ tree; the manifest exports 21 and
every other doc already listed it.

API-REFERENCE.md labelled endpoint scopes two ways — **Required scope** on the
13 older endpoints, **Required API scope(s)** on the 5 Reports ones added after
the rule was written. No scope information was missing, but the contract states
the requirement in terms of the newer label, so a grep for it read as 13 gaps.
Normalised to one spelling.

Also: a dead #code-style anchor in CONTRIBUTING.md (the heading is "Code style
and consistency"), and the two report-rendering bullets in CMDLET-REFERENCE.md
now mention that code blocks keep their indentation and scroll horizontally.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Copilot AI lite review requested due to automatic review settings September 11, 2026 14:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved decoder coverage and profile-rendering/type-detection issues remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Updates report rendering for base64-encoded macOS/iOS profiles and script content, alongside export, documentation, test, and version changes.

Changes:

  • Centralizes validated base64 text decoding.
  • Adds XML rendering, horizontal scrolling, and export cleanup.
  • Updates tests, documentation, changelog, and version to 0.7.1.
File summaries
File Change
Tests/Consistency.Tests.ps1 Adds decoding and renderer regression coverage.
module/README.md Corrects the exported cmdlet count and listing.
module/Private/Get-InforcerComparisonData.ps1 Uses shared decoding logic.
module/Private/Export-InforcerDocExcel.ps1 Removes internal render markers.
module/Private/ConvertTo-InforcerSettingRows.ps1 Decodes textual payload fields.
module/Private/ConvertTo-InforcerMarkdown.ps1 Handles newlines and removes markers.
module/Private/ConvertTo-InforcerHtml.ps1 Adds XML rendering and scrolling.
module/Private/ConvertTo-InforcerComparisonHtml.ps1 Adds comparison XML rendering.
module/Private/ConvertFrom-InforcerBase64Text.ps1 Centralizes base64 text validation.
module/Private/Compare-InforcerDocModels.ps1 Applies shared decoding during comparisons.
module/InforcerCommunity.psd1 Bumps the module version to 0.7.1.
docs/CMDLET-REFERENCE.md Documents rendering behavior.
docs/API-REFERENCE.md Normalizes API scope labels.
CONTRIBUTING.md Fixes the code-style anchor.
CHANGELOG.md Records the 0.7.1 changes.
Review details

Suppressed comments (6)

module/Private/ConvertFrom-InforcerBase64Text.ps1:10

  • The new helper is not used by every existing report decode path: module/Private/Resolve-InforcerGraphEnrichment.ps1:146-154 still calls lenient Encoding.UTF8.GetString(FromBase64String(...)) for Graph-fetched rulesContent. Graph-enriched comparisons therefore bypass the strict UTF-8/control-byte guard and can still emit replacement characters, so route that call through this helper too.
        Intune base64-encodes both text (scriptContent, rulesContent, the macOS/iOS
        .mobileconfig payload plist) and binary (hashedScriptContent digests, signed
        profiles) — so the property name alone is not a safe test. This decodes and
        verifies the bytes were text, returning $null otherwise so callers keep the
        base64 as-is rather than rendering mojibake.

module/Private/ConvertTo-InforcerComparisonHtml.ps1:1351

  • This XML branch is only reached for values in the manual-review renderer. macOS/iOS configuration-profile categories are not routed to manual review, while the normal comparison-row renderer strips __SCRIPT_CODE__ and emits the payload as a plain/truncated table value. Therefore Compare-InforcerEnvironments still does not provide the promised collapsible, XML-highlighted profile; route configuration-profile categories through this path or add XML block rendering to normal comparison rows, with a regression test.
                        elseif ($trimmedCode -match '^\s*<') { $codeClass = 'xml-code'; $summaryClass = 'xml-code-summary'; $codeLabel = 'View profile' }

module/Private/ConvertTo-InforcerComparisonHtml.ps1:1351

  • Please add a regression test for the new XML branch in ConvertTo-InforcerComparisonHtml using a manual-review payload value. The existing comparison-renderer tests cover PowerShell/Bash/JSON paths, but they do not exercise xml-code, View profile, or highlightXML, so a future break in the newly added comparison report path would not be caught.
                        elseif ($trimmedCode -match '^\s*<') { $codeClass = 'xml-code'; $summaryClass = 'xml-code-summary'; $codeLabel = 'View profile' }

module/Private/ConvertTo-InforcerComparisonHtml.ps1:1351

  • This condition treats any decoded value beginning with < as a profile. A PowerShell script commonly starts with a <# block comment, so it will now be labeled View profile and receive XML highlighting instead of script highlighting. Match a plist/XML signature such as <?xml, <plist, or <!DOCTYPE plist rather than any <.
                        elseif ($trimmedCode -match '^\s*<') { $codeClass = 'xml-code'; $summaryClass = 'xml-code-summary'; $codeLabel = 'View profile' }

module/Private/ConvertTo-InforcerHtml.ps1:998

  • This condition treats any decoded value beginning with < as a profile. A PowerShell script commonly starts with a <# block comment, so it will now be labeled View profile and receive XML highlighting instead of script highlighting. Match a plist/XML signature such as <?xml, <plist, or <!DOCTYPE plist rather than any <.
                            elseif ($trimmed -match '^\s*<') { $codeClass = 'xml-code'; $summaryClass = 'xml-code-summary'; $label = 'View profile' }

module/Private/ConvertTo-InforcerSettingRows.ps1:335

  • payload is marked as __SCRIPT_CODE__, but categories such as macOS / Configuration Profiles do not satisfy the manual-review routing predicate in Compare-InforcerDocModels.ps1:461-515. Those rows use the ordinary comparison table, where the marker is only stripped; the new XML branch is never reached, so comparison reports lose the collapsible XML block and their newlines collapse. Route payload-bearing non-catalog policies to the code-block path or make comparison rows marker-aware.
            # Decode scripts, rulesContent JSON and .mobileconfig payloads. hashedScriptContent
            # matches this pattern too but is a digest — the helper rejects it.
            if ($prop.Name -match '(?i)scriptContent|rulesContent|^payload$' -and $strVal -is [string]) {
                $decoded = ConvertFrom-InforcerBase64Text -Value $strVal
                if ($decoded) { $strVal = "__SCRIPT_CODE__$decoded" }
  • Files reviewed: 15/15 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread module/Private/ConvertFrom-InforcerBase64Text.ps1 Outdated
From Copilot's PR review, verified before acting: strict UTF-8 decoding is
necessary but not sufficient. EF BF BD is a *valid* encoding of U+FFFD and
C2 80..C2 9F validly encode the C1 controls, so neither throws — a digest
carrying those bytes passed the guard and came back as "text".

This also exposed a hole in the test I wrote: "never returns a string containing
U+FFFD" passed only because its 0..255 byte array is invalid UTF-8, not because
U+FFFD was rejected. Name and behaviour had drifted apart.

Two tests added for the cases that actually slipped through, both mutation-
verified against the previous guard, plus one that tab/CR/LF still survive so
the tighter range can't quietly start rejecting real scripts.

451/0/2.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Copilot AI review requested due to automatic review settings September 11, 2026 14:41
@royklo
royklo merged commit 616e0fd into main Sep 11, 2026
4 checks passed
@royklo
royklo deleted the fix/decode-macos-payloads-and-hashes branch September 11, 2026 14:42

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Eight unresolved moderate findings affect decoding, comparison rendering, and Markdown export.

Review details

Suppressed comments (8)

Previously missed (1) — in code that hasn't changed since the last review.

module/Private/ConvertTo-InforcerMarkdown.ps1:48

  • Decoded plist content is inserted into Markdown table cells without escaping <, > or &. GFM treats tags such as <plist>, <dict>, and <key> as raw HTML, so the Markdown export will not preserve the profile as literal XML (and an XML declaration may be interpreted as a processing instruction). HTML-escape the cell value before replacing newlines so decoded profiles remain readable and safe.

module/Private/ConvertFrom-InforcerBase64Text.ps1:25

  • This length gate rejects valid short text fields. For example, a script body of echo hi encodes as ZWNobyBoaQ== (12 characters), so the helper returns $null and the caller leaves scriptContent as base64 instead of rendering the script. Since this helper is now the common decoder for all script fields, please remove this floor or replace it with a validation that does not discard short valid payloads.
    if ([string]::IsNullOrWhiteSpace($Value) -or $Value.Length -le 20 -or $Value -match '\s') { return $null }

module/Private/ConvertTo-InforcerComparisonHtml.ps1:1351

  • The new XML path in the comparison renderer is not covered by the added tests: the plist test exercises ConvertTo-InforcerHtml, while no test renders ConvertTo-InforcerComparisonHtml with a __SCRIPT_CODE__<plist... setting. Because this renderer has its own XML classification, CSS, and highlighter, a regression here could pass the suite; add a comparison fixture/assertion for the XML block and preserved profile content.
                        elseif ($trimmedCode -match '^\s*<') { $codeClass = 'xml-code'; $summaryClass = 'xml-code-summary'; $codeLabel = 'View profile' }

module/Private/ConvertTo-InforcerComparisonHtml.ps1:1351

  • This XML branch is only reached for __SCRIPT_CODE__ values in the Manual Review cards. Standard macOS / Configuration Profiles categories are not routed to ManualReview by Compare-InforcerDocModels, so their decoded payload remains in the main comparison table and is displayed as plain/truncated cell text rather than a collapsible, XML-highlighted profile. Route payload profiles to manual review or add payload-aware rendering to comparison rows.
                        elseif ($trimmedCode -match '^\s*<') { $codeClass = 'xml-code'; $summaryClass = 'xml-code-summary'; $codeLabel = 'View profile' }

module/Private/ConvertTo-InforcerComparisonHtml.ps1:1351

  • The same < prefix check misclassifies PowerShell scripts starting with <# in comparison reports, so those scripts are shown as profiles with XML highlighting. Use the same XML-specific prefix check as the tenant renderer rather than treating every leading < as plist XML.
                        elseif ($trimmedCode -match '^\s*<') { $codeClass = 'xml-code'; $summaryClass = 'xml-code-summary'; $codeLabel = 'View profile' }

module/Private/ConvertTo-InforcerHtml.ps1:998

  • The new < prefix check also matches PowerShell block-comment prologues such as <# comment-based help #>. A script beginning with comment-based help will therefore be labeled View profile and receive XML highlighting instead of the existing script rendering. Restrict this branch to an XML declaration/DOCTYPE or a plist root so script content is not misclassified.
                            elseif ($trimmed -match '^\s*<') { $codeClass = 'xml-code'; $summaryClass = 'xml-code-summary'; $label = 'View profile' }

module/Private/ConvertTo-InforcerMarkdown.ps1:48

  • The decoder explicitly preserves bare CR characters, but this replacement only handles CRLF and LF. A decoded script or plist containing a lone \r can still terminate a GFM row; normalize all line-ending forms before emitting the table.
            $val = $val -replace '\r?\n', '<br>'

module/Private/Get-InforcerComparisonData.ps1:226

  • This centralizes decoding for the linked-script path, but Resolve-InforcerGraphEnrichment.ps1 still decodes Graph rulesContent with Encoding.UTF8.GetString(FromBase64String(...)). That lenient path can still emit U+FFFD for invalid bytes when -FetchGraphData is used, so the no-mojibake guarantee and stated replacement of all duplicate decoders are incomplete; route that call through this helper too.
                        $decoded = ConvertFrom-InforcerBase64Text -Value $val
                        if ($decoded) { $val = $decoded }
  • Files reviewed: 15/15 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants